Skip to content

fix(ui5-select): prevent crash on ArrowUp/Down when value matches no option #12094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kgogov
Copy link
Member

@kgogov kgogov commented Aug 8, 2025

If select's value property is set to a string that doesn’t match any option, nothing is selected (selectedIndex = -1). When you press ArrowUp or ArrowDown, _changeSelectedItem tries to use invalid indices, which causes it to access undefined and crash at runtime.

Fixes: #12093

@NakataCode NakataCode self-requested a review August 14, 2025 10:34
@kgogov kgogov force-pushed the select-keyboard-navigation-fix branch from 1e97c10 to 68c4f46 Compare August 15, 2025 13:11
</Select>
);

cy.get("#sel-down")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the id attribute from the mount and use [ui5-select] selector instead of #sel-down throughout the test

</Select>
);

cy.get("#sel-up")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here in this test, you can use [custom-tag] instead of the id(and remove the id from the mount)

);

cy.get("#sel-down")
.should("have.attr", "value", "missing")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, consider using have.prop for the value assertion since you're testing the component's value property:

.should("have.prop", "value", "missing")

);

cy.get("#sel-up")
.should("have.attr", "value", "missing")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, you can change it to have.prop for consistency

kgogov added 2 commits August 15, 2025 17:06
…option

If select's value property is set to a string that doesn’t match any option,
nothing is selected (selectedIndex = -1). When you press ArrowUp or
ArrowDown, _changeSelectedItem tries to use invalid indices, which
causes it to access undefined and crash at runtime.

Fixes: #12093
@kgogov kgogov force-pushed the select-keyboard-navigation-fix branch from 68c4f46 to c6ce6f2 Compare August 15, 2025 14:07
@kgogov kgogov requested a review from NakataCode August 15, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui5-select]: keyboard navigation throwing exception
2 participants